Skip to content

Adds a fast basket endpoint and functions - #21

Open
ajrbyers wants to merge 273 commits into
mainfrom
b-fast-basket
Open

Adds a fast basket endpoint and functions#21
ajrbyers wants to merge 273 commits into
mainfrom
b-fast-basket

Conversation

@ajrbyers

Copy link
Copy Markdown
Contributor

No description provided.

MartinPaulEve and others added 30 commits January 15, 2024 17:26
…d of fetching all works to improve performance
…package prices

This commit adds a new management command `pregenerate_prices.py` that pregenerates package prices. The command retrieves a full list of countries that have currencies and calculates the minimum and maximum prices for each package and country combination. The results are stored in a dictionary `package_list` and printed to the console. This command will be useful for generating price data for packages in advance.
…ew to improve code readability and maintainability
…ew to improve code readability and maintainability
…play multiple contributors and their institutions
…ormance and reduce database queries in all_books view
…int to 'summary_meta_package' instead of 'summary_package_initiative' for consistency and clarity
…m the dropdown menu

🔧 fix(views.py): remove commented out code that was no longer needed
…rove performance and reduce database queries in ThothWorkManager

🔀 refactor(views.py): remove unnecessary filter condition in all_books view
…obile view to improve readability and user experience
…o improve readability and aesthetics

🎨 style(style.css): update font family and size for various elements to improve consistency and aesthetics
✨ feat(index.html): add pill buttons to build subscription, search catalogue, and apply for membership sections for better user experience
Add a new option to filter initiatives from the selectable list.
model_name='bandingtypecurrencyentry',
index=models.Index(fields=['package', 'banding_type_entry'], name='btce_pkg_bte_idx'),
),
migrations.AddIndex(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure how helpful this index will be, as the bandingtypeentry is probably shared by many rows

Comment thread src/package/templatetags/country.py
Comment thread src/package/forms.py
else:
self.helper.layout.append(
Layout(
HTML(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat curious offloading the HTML generation into this routine, rather than using a template?

Comment thread src/package/models.py
return prices

@functools.cached_property
def fast_price_bandings(self) -> dict:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK, but I really would like it to have a debug/logging mode where it writes output about what it is doing to the price debugger. Could just add an optional parameter to the function debug=False that when true then returns a tuple of (prices, debug_string). This way we could easily hook it up to the debugger? I am just worried that, although it all looks fine at this point, we might hit an edge case with OBC where we can't work out what it's doing and need to debug the logic.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. This might be the wrong place for this comment. I can see that other functions still have the "explain" bool signature, below, so perhaps the debugger is still in tact

Comment thread src/package/tests.py
def _compare(self, package):
original = package.price_bandings

if "fast_price_bandings" in package.__dict__:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code is forcing a cached property to reload, but it's hard to understand. It looks like the code deletes an instance attribute but then accesses it immediately...

Comment thread src/package/tests.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants